Condition PSScriptAnalyzer CI Claim on Repos Carrying .ps1 Files - #686
Merged
Conversation
GOVERNANCE.md's Running the Linters Locally bullet unconditionally claimed CI runs PSScriptAnalyzer and linked PSScriptAnalyzerSettings.psd1, true for the hub itself but not for every downstream repo. Hit as a real Copilot finding on both Blog#94 (a stale carried validate-task.yml never wired the step) and HomeAutomation-Config#50 (the repo carries no .ps1 files at all, so the link 404s). Neither is fixable per-repo without either breaking byte-parity on the verbatim bullet or inventing content the repo does not need, so both were replied-and-deferred as the maintainer's call. Condition both bullets on a repo carrying .ps1 files instead, closing the recurring gap at its source. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates governance documentation to accurately describe when PSScriptAnalyzer is expected to run in CI and how to invoke it locally, avoiding incorrect universal claims for downstream repos that do not carry PowerShell scripts.
Changes:
- Condition the CI lint-surface description of PSScriptAnalyzer on repositories that carry
*.ps1files. - Condition the local PSScriptAnalyzer invocation guidance similarly, clarifying applicability.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Round 1 review: the surrounding prose became conditional on a repo carrying .ps1 files, but the Markdown hyperlink to PSScriptAnalyzerSettings.psd1 stayed unconditional and still 404s when carried into a repo without the file. Switch to inline code, no link. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
GOVERNANCE.md:242
- The PSScriptAnalyzer invocation still unconditionally passes
-Settings ./PSScriptAnalyzerSettings.psd1, but the updated prose now allows for repos that carry.ps1files without necessarily carrying that settings file. In that case this “known-working invocation” will fail immediately. Consider making the command gracefully fall back to default rules when the settings file is absent (with a warning).
- **PSScriptAnalyzer** (PowerShell, the peer of the shellcheck step, **only applies to a repo that carries `.ps1` files**, with the excluded rules and their reasons in `PSScriptAnalyzerSettings.psd1` alongside a repo that carries one):
```sh
docker run --rm --pull=always -e PS_SCRIPTS="$(git ls-files '*.ps1')" -v "$PWD":/mnt --workdir /mnt mcr.microsoft.com/powershell:latest \
pwsh -NoProfile -Command '
Round 2 review (suppressed): the invocation still unconditionally passed -Settings ./PSScriptAnalyzerSettings.psd1, but nothing said a repo carrying .ps1 files also carries that settings file, so a repo with scripts but no settings file would hit an immediate hard failure. State the pairing explicitly rather than adding fallback logic the rest of this doc's required-baseline-file pattern does not use elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
Suppressed finding (GOVERNANCE.md:242, the unconditional |
ptr727
added a commit
that referenced
this pull request
Aug 15, 2026
…Python CI Gates (#718) Thirty-one squashes, `56f4d7d..d54862a`. 115 files, +20436/-5298. **Merge with a merge commit, never a squash, and never with `--delete-branch`.** This pull request's head is `develop` itself. ## What lands **Fleet Skills.** The `.agents/skills/` source tree, the generated `.claude-plugin/` distribution, `scripts/build_dist.py` with its `--check` gate, and `scripts/skills_install.py` with its host stamp (#676). Packaged as skills on top of the scaffold: PR review conduct and Copilot instructions upkeep (#677), comment and doc style (#678), resync-a-repo and fleet-conformance-check (#679), the per-language codestyles (#680), git commit conventions and operational vs release workflow (#681), stand up a repo (#683), and repo-worktree (#717). Coverage gaps closed in three passes (#690, #691, #692) plus the P4 sentence-length opt-in (#697). **Host setup.** The Windows host-setup tooling and its PowerShell gate (#674), the Windows bootstrap loader (#682), Docker install and upgrade on Linux and Windows with a version floor (#701, #705), a `uv` floor in `spec/host-tools.json` (#698), self-healing of a shadowing `uv`, `jq`, or `git-restore-mtime` copy (#689), node's real winget package id (#696), and a README for the Linux host-setup nuances (#710). **Python and CI.** Python tooling in CI with the script tests moved to `scripts/tests` (#704), `ruff format` adopted and gated (#709), and the PSScriptAnalyzer claim conditioned on repos that carry `.ps1` files (#686). **Conduct rules.** Triage-order and scope guardrails in pr-review-conduct (#684), `pr_review.py wait` requesting a review rather than only polling for one (#685), a tech-agnostic signed-commit verification (#708), execution rather than analogy to verify platform-specific code (#715), and a unique worktree for every task (#717). **Docs.** The fleet map and gap register with peer messaging declared (#687), mermaid flow diagrams in the kept-authority docs (#702), and the map pointed at the shipped diagrams and current tooling (#703). ## Issues this promotion closes Each landed on `develop` on its own pull request. The keyword fires only on a merge into `main`, so it sits here rather than on the feature pull requests. Closes #700 Closes #707 Closes #711 Closes #712 Closes #714 Closes #688 #699 stays open on purpose: #717 shipped the layout convention and the skill, and the physical migration of existing checkouts is still tracked there. ## Review record Every squash closed its own Copilot loop on its own pull request before merging to `develop`. This promotion carries no new content of its own, so its review is the merged tree as a whole. ## Consequence worth stating The `GOVERNANCE.md` and `AGENTS.md` sections these squashes changed become the canonical the moment this reaches `main`, and every carrying repository reads as drifted from that point until it resyncs. That is the ordinary consequence of a canonical moving rather than a defect. The Skills installer added here is also how a machine picks the new skills up, so a session that keeps restating a rule already packaged as a skill is the signal to run it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GOVERNANCE.md's "Running the Linters Locally" bullet unconditionally claimed CI runs PSScriptAnalyzer and linked
PSScriptAnalyzerSettings.psd1, true for the hub itself but not for every downstream repo.Hit as a real Copilot finding on both Blog#94 (a stale carried
validate-task.ymlnever wired the step) and HomeAutomation-Config#50 (the repo carries no.ps1files at all, so the link 404s). Neither was fixable per-repo without either breaking byte-parity on the verbatim bullet or inventing content the repo does not need, so both were replied-and-deferred as the maintainer's call.This conditions both bullets on a repo carrying
.ps1files instead, closing the recurring gap at its source rather than continuing to defer it repo by repo through the rest of the Phase 8 rollout.Not manifest-tracked ("Running the Linters Locally" is not a declared verbatim section in
spec/files.json), so this is a hub-only prose fix with no spec/audit coupling to update.🤖 Generated with Claude Code